feat: wire document update to MCP, CLI, and REST API (#182)#270
feat: wire document update to MCP, CLI, and REST API (#182)#270
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds support for updating existing documents across LibScope’s interfaces (core export, MCP tool, CLI command, and REST API), including re-chunking/re-embedding when content changes.
Changes:
- Add
updateDocumentto public core exports and wire it into MCP and CLI. - Add REST
PUT/PATCH /api/v1/documents/:idroute to perform partial document updates. - Add unit tests and CLI reference docs for the new update flow.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/documents.test.ts | Adds unit tests covering updateDocument update/validation scenarios |
| src/mcp/server.ts | Exposes update-document MCP tool and maps parameters to updateDocument |
| src/core/index.ts | Exports updateDocument and UpdateDocumentInput from core |
| src/cli/index.ts | Adds libscope docs update <id> command that calls updateDocument |
| src/api/routes.ts | Adds PUT/PATCH handler for /api/v1/documents/:id using updateDocument |
| docs/reference/cli.md | Documents the new CLI docs update command |
| withErrorHandling(async (params) => { | ||
| const metadata: Record<string, string | null | undefined> = {}; | ||
| if (params.library !== undefined) metadata.library = params.library; | ||
| if (params.version !== undefined) metadata.version = params.version; | ||
| if (params.url !== undefined) metadata.url = params.url; | ||
| if (params.topicId !== undefined) metadata.topicId = params.topicId; | ||
|
|
There was a problem hiding this comment.
This tool allows calls with only documentId and no update fields; that will still create a new version / bump timestamps even though nothing changed. Consider validating that at least one of title, content, or the metadata fields is provided and return a ValidationError otherwise.
docs/reference/cli.md
Outdated
| ## Document Links (Cross-references) | ||
|
|
There was a problem hiding this comment.
The libscope docs update docs are inserted under the “Document Links (Cross-references)” section and also cause that header to appear twice. This should likely live under “Document Management” (near list/show/delete/history) and the duplicate “Document Links” header should be removed to keep the structure consistent.
| ## Document Links (Cross-references) |
| const updated = await updateDocument(vecDb, provider, indexed.id, { | ||
| content: "Brand new content that is different.", | ||
| }); | ||
|
|
||
| expect(updated.content).toBe("Brand new content that is different."); | ||
| // Verify chunks were recreated | ||
| const chunks = vecDb | ||
| .prepare("SELECT id FROM chunks WHERE document_id = ?") | ||
| .all(indexed.id) as { id: string }[]; | ||
| expect(chunks.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
The “should update content and re-chunk” assertion only checks that at least one chunk exists after the update, which would also pass if updateDocument did not delete/recreate chunks (because chunks already exist from the initial index). Strengthen this test to verify re-chunking actually happened (e.g., compare old vs new chunk IDs/content, verify chunk count changes appropriately, and/or confirm chunk_embeddings rows were replaced).
| const updated = await updateDocument(vecDb, provider, indexed.id, { | |
| content: "Brand new content that is different.", | |
| }); | |
| expect(updated.content).toBe("Brand new content that is different."); | |
| // Verify chunks were recreated | |
| const chunks = vecDb | |
| .prepare("SELECT id FROM chunks WHERE document_id = ?") | |
| .all(indexed.id) as { id: string }[]; | |
| expect(chunks.length).toBeGreaterThan(0); | |
| // Capture original chunks and their embeddings | |
| const originalChunks = vecDb | |
| .prepare( | |
| "SELECT id, content FROM chunks WHERE document_id = ? ORDER BY id", | |
| ) | |
| .all(indexed.id) as { id: string; content: string }[]; | |
| expect(originalChunks.length).toBeGreaterThan(0); | |
| const originalChunkIds = originalChunks.map((c) => c.id); | |
| const originalEmbeddings = vecDb | |
| .prepare( | |
| "SELECT chunk_id FROM chunk_embeddings WHERE document_id = ? ORDER BY chunk_id", | |
| ) | |
| .all(indexed.id) as { chunk_id: string }[]; | |
| expect(originalEmbeddings.length).toBeGreaterThan(0); | |
| const updated = await updateDocument(vecDb, provider, indexed.id, { | |
| content: "Brand new content that is different.", | |
| }); | |
| expect(updated.content).toBe("Brand new content that is different."); | |
| // Verify chunks were recreated: new chunks exist and have different IDs | |
| const newChunks = vecDb | |
| .prepare( | |
| "SELECT id, content FROM chunks WHERE document_id = ? ORDER BY id", | |
| ) | |
| .all(indexed.id) as { id: string; content: string }[]; | |
| expect(newChunks.length).toBeGreaterThan(0); | |
| const newChunkIds = newChunks.map((c) => c.id); | |
| // Ensure none of the original chunk IDs are reused | |
| expect( | |
| newChunkIds.every((id) => !originalChunkIds.includes(id)), | |
| ).toBe(true); | |
| // Verify embeddings now correspond to the new chunks | |
| const newEmbeddings = vecDb | |
| .prepare( | |
| "SELECT chunk_id FROM chunk_embeddings WHERE document_id = ? ORDER BY chunk_id", | |
| ) | |
| .all(indexed.id) as { chunk_id: string }[]; | |
| expect(newEmbeddings.length).toBeGreaterThan(0); | |
| const newEmbeddingChunkIds = newEmbeddings.map((e) => e.chunk_id); | |
| const sortedNewChunkIds = [...newChunkIds].sort(); | |
| const sortedNewEmbeddingChunkIds = [...newEmbeddingChunkIds].sort(); | |
| expect(sortedNewEmbeddingChunkIds).toEqual(sortedNewChunkIds); |
| sendError(res, 400, "VALIDATION_ERROR", "Request body must be a JSON object"); | ||
| return; | ||
| } | ||
| const b = body as Record<string, unknown>; |
There was a problem hiding this comment.
PUT/PATCH currently accept an empty JSON object (or a body with no supported fields) and still call updateDocument, which will create a new version / update timestamps despite being a no-op. Consider validating that at least one updatable field is present and return 400 for empty updates (matches the issue’s “invalid/empty body” requirement).
| const b = body as Record<string, unknown>; | |
| const b = body as Record<string, unknown>; | |
| const hasUpdatableField = | |
| "title" in b || | |
| "content" in b || | |
| "library" in b || | |
| "version" in b || | |
| "url" in b || | |
| "topicId" in b; | |
| if (!hasUpdatableField) { | |
| sendError( | |
| res, | |
| 400, | |
| "VALIDATION_ERROR", | |
| "Request body must contain at least one updatable field", | |
| ); | |
| return; | |
| } |
| if (b["library"] !== undefined) | ||
| metadata.library = typeof b["library"] === "string" ? b["library"] : null; | ||
| if (b["version"] !== undefined) | ||
| metadata.version = typeof b["version"] === "string" ? b["version"] : null; | ||
| if (b["url"] !== undefined) metadata.url = typeof b["url"] === "string" ? b["url"] : null; | ||
| if (b["topicId"] !== undefined) | ||
| metadata.topicId = typeof b["topicId"] === "string" ? b["topicId"] : null; |
There was a problem hiding this comment.
For library/version/url/topicId, if the field is present but not a string (e.g. 123), this code coerces it to null, which will silently clear the stored value. It would be safer to treat “present but not string or null” as a validation error (400) to avoid unintended data loss.
| if (b["library"] !== undefined) | |
| metadata.library = typeof b["library"] === "string" ? b["library"] : null; | |
| if (b["version"] !== undefined) | |
| metadata.version = typeof b["version"] === "string" ? b["version"] : null; | |
| if (b["url"] !== undefined) metadata.url = typeof b["url"] === "string" ? b["url"] : null; | |
| if (b["topicId"] !== undefined) | |
| metadata.topicId = typeof b["topicId"] === "string" ? b["topicId"] : null; | |
| const library = b["library"]; | |
| if (library !== undefined) { | |
| if (library !== null && typeof library !== "string") { | |
| sendError(res, 400, "VALIDATION_ERROR", "Field 'library' must be a string or null"); | |
| return; | |
| } | |
| metadata.library = library as string | null; | |
| } | |
| const version = b["version"]; | |
| if (version !== undefined) { | |
| if (version !== null && typeof version !== "string") { | |
| sendError(res, 400, "VALIDATION_ERROR", "Field 'version' must be a string or null"); | |
| return; | |
| } | |
| metadata.version = version as string | null; | |
| } | |
| const url = b["url"]; | |
| if (url !== undefined) { | |
| if (url !== null && typeof url !== "string") { | |
| sendError(res, 400, "VALIDATION_ERROR", "Field 'url' must be a string or null"); | |
| return; | |
| } | |
| metadata.url = url as string | null; | |
| } | |
| const topicId = b["topicId"]; | |
| if (topicId !== undefined) { | |
| if (topicId !== null && typeof topicId !== "string") { | |
| sendError(res, 400, "VALIDATION_ERROR", "Field 'topicId' must be a string or null"); | |
| return; | |
| } | |
| metadata.topicId = topicId as string | null; | |
| } |
| const { db, provider } = initializeAppWithEmbedding(); | ||
| try { | ||
| const metadata: Record<string, string | null | undefined> = {}; | ||
| if (opts.library !== undefined) metadata.library = opts.library; | ||
| if (opts.version !== undefined) metadata.version = opts.version; | ||
| if (opts.url !== undefined) metadata.url = opts.url; |
There was a problem hiding this comment.
The CLI command can be invoked with no flags (only <documentId>), which will call updateDocument with no changes and still create a new version / bump updated_at. Add a guard to require at least one of the update options and show a helpful error when none are provided.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
53cf4e3 to
49759ff
Compare
Closes #182
Exposes the existing
updateDocument()function through all interfaces:update-document— update title, content, library, version, url, topicIdlibscope docs update <id>with--title,--content,--library,--version,--url,--topicPUT/PATCH /api/v1/documents/:id— partial updates